8314323: Implement JEP 527: TLS 1.3 Hybrid Key Exchange #27614

haimaychao · 2025-10-02T22:52:49Z

Implement hybrid key exchange support for TLS 1.3 by adding three post-quantum hybrid named groups: X25519MLKEM768, SecP256r1MLKEM768, and SecP384r1MLKEM1024.
Please see JEP 527 for details about this change.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8369848 to be approved

Issues

JDK-8314323: Implement JEP 527: TLS 1.3 Hybrid Key Exchange (Enhancement - P2)
JDK-8369848: Implement JEP 527: TLS 1.3 Hybrid Key Exchange (CSR)

Contributors

Jamil Nimeh <[email protected]>
Weijun Wang <[email protected]>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27614/head:pull/27614
$ git checkout pull/27614

Update a local copy of the PR:
$ git checkout pull/27614
$ git pull https://git.openjdk.org/jdk.git pull/27614/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27614

View PR using the GUI difftool:
$ git pr show -t 27614

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27614.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-10-02T22:53:45Z

👋 Welcome back hchao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-10-02T22:54:03Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-10-02T22:54:49Z

@haimaychao The following label will be automatically applied to this pull request:

security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

haimaychao · 2025-10-02T22:56:54Z

/contributor add @jnimeh

haimaychao · 2025-10-02T22:57:21Z

/contributor add @wangweij

openjdk · 2025-10-02T22:57:41Z

@haimaychao
Contributor Jamil Nimeh <[email protected]> successfully added.

openjdk · 2025-10-02T22:58:24Z

@haimaychao
Contributor Weijun Wang <[email protected]> successfully added.

mlbridge · 2025-10-03T00:03:19Z

Webrevs

artur-oracle · 2025-10-03T14:19:44Z

src/java.base/share/classes/sun/security/ssl/NamedGroup.java

-                algParams.init(keAlgParamSpec);
+                // Skip AlgorithmParameters for KEMs (not supported)
+                if (namedGroupSpec == NamedGroupSpec.NAMED_GROUP_KEM) {
+                    if (defaultProviderName == null) {


We assume that if provider is not null then it must be DH without doing any checks to confirm that. It would be cleaner to call getProvider() instead.

Provider p = getProvider();
if (p == null) {
KeyFactory.getInstance(name);
} else {
KeyFactory.getInstance(name, p);
}

It's actually unchanged, do you plan to do it in the next iteration?

Sorry. Changed to call getProvider() as suggested.

artur-oracle · 2025-10-03T16:51:17Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+
+    private static KEM getKEM(String name) throws NoSuchAlgorithmException {
+        if (name.startsWith("secp") || name.equals("X25519") ||
+                name.equals("X448")) {


Do we need X448 here? Also, please provide a comment for this method with functionality description.

X448 removed, and comment added.

artur-oracle · 2025-10-03T16:53:00Z

src/java.base/share/classes/com/sun/crypto/provider/DH.java

+        X25519(32, 32,
+                "XDH", "XDH", NamedParameterSpec.X25519),
+
+        X448(56, 56,


Why do we need X448 and P521?

Need, no. Want, yes. The support for traditional curves that are not part of the first round of hybrid KEMs lays the groundwork for future hybrid KEMs that might use these larger curves. It also gives us the base framework to move these algorithms as named groups to KEM implementations in the future.

I see, thanks for the explanation! I guess it makes sense if we expect those curves to be used in the future rounds of hybrid KEM.

artur-oracle · 2025-10-03T17:22:36Z

src/java.base/share/classes/com/sun/crypto/provider/DH.java

+            implements KEMSpi.EncapsulatorSpi, KEMSpi.DecapsulatorSpi {
+        private final PublicKey pkR;
+        private final PrivateKey skR;
+        private final AlgorithmParameterSpec spec;


This variable is not being used, it can be removed together with the constructor parameter.

Removed variable spec.

ecki · 2025-10-03T17:10:01Z

src/java.base/share/classes/com/sun/crypto/provider/DH.java

+                }
+            } else if (k instanceof XECKey xkey
+                    && xkey.getParams() instanceof NamedParameterSpec ns) {
+                if (ns.getName().equalsIgnoreCase("X25519")) {


Maybe use NamedParameterSpec.X25519?

ecki · 2025-10-03T17:15:33Z

src/java.base/share/classes/sun/security/ssl/NamedGroup.java

            ProtocolVersion.PROTOCOLS_TO_13,
            PredefinedDHParameterSpecs.ffdheParams.get(8192)),

+    ML_KEM_512(0x0200, "MLKEM512",


Are they needed for this Jep?

We added ML-KEM NamedGroups with null AlgorithmParameterSpec, and they won’t appear as negotiable named groups. They were added to support debug display and recognition of MLKEM named groups when used in the key share, so we can see them in debug and know if they are used. It'd help for interop debugging/testing.

ecki · 2025-10-03T17:17:57Z

src/java.base/share/classes/sun/security/ssl/NamedGroup.java

        // Finite Field Groups (XDH)
        NAMED_GROUP_XDH("XDH", XDHScheme.instance),

+        NAMED_GROUP_KEM("PQC", KEMScheme.instance),


That Choice of Name needs probably an explaining comment if it is for pure PQC and/ormhybrid?

Comment added.

Maybe we can rename "PQC" to "KEM" to be consistent.

Rename done.

ecki · 2025-10-03T17:20:04Z

src/java.base/share/classes/sun/security/ssl/NamedGroup.java

            } else {        // default groups
                NamedGroup[] groups = new NamedGroup[] {

+                        // Hybrid key agreements


It feels like all the infra for X448MLKEM1024 is there, so rather than removing x448 from this patch, why not implement it (it’s more obvious than P511 Variants)

Indeed the infrastructure is there, but I don't see an IETF draft that covers that hybrid variant for TLS, nor do I see an IANA mapping for it here: https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8

There needs to be a standard for TLS 1.3 backing these hybrid KEMs before we implement them.

ecki · 2025-10-03T17:20:32Z

src/java.base/share/classes/sun/security/ssl/NamedGroup.java

                        FFDHE_2048,
                        FFDHE_3072,
                        FFDHE_4096,
-                        FFDHE_6144,


Unrelated change?

No, the choise to knock out ffdhe6144 and 8192 from the default list was done on purpose. I don't think they get much use and they can always be re-enabled via SSLParameters or the system property. We're open to feedback on this if you or others feel like they should remain in place, though.

The change is I think ok, doesn’t make much of a difference for most cases I was just thinking it needed its own commit and ticket reference but if it was intentional fine as well.

I think you make a fair point here. It probably deserves its own change, JBS entry, CSR, etc. We'll leave them in for now.

ffdhe6144 and 8192 added.

ecki · 2025-10-03T17:23:37Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+    }
+
+    private static AlgorithmParameterSpec getSpec(String name) {
+        if (name.startsWith("secp")) {


This seems to be repeated multiple times including the case sensitive string, maybe have an APS.isGenericEC() helper?

ecki · 2025-10-03T17:26:01Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+
+        private static int leftPublicLength(String name) {
+            return switch (name.toLowerCase(Locale.ROOT)) {
+                case "secp256r1" -> 65;


Isn’t that Part formte named groups, maybe Look it up instead?

ecki · 2025-10-03T17:27:27Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+
+    private static byte[] concat(byte[]... inputs) {
+        ByteArrayOutputStream o = new ByteArrayOutputStream();
+        Arrays.stream(inputs).forEach(o::writeBytes);


Do we really want a non presized buffer and a stream in the handshake hot path?

Updated to use presized buffer.

artur-oracle · 2025-10-06T13:57:48Z

test/jdk/sun/security/ssl/CipherSuite/DisabledCurve.java

 * @run main/othervm -Djdk.tls.namedGroups="secp384r1"
        DisabledCurve DISABLE_NONE PASS
 * @run main/othervm -Djdk.tls.namedGroups="secp384r1"
        DisabledCurve secp384r1 FAIL


This test case fails whether we specify -Djdk.tls.namedGroups="secp384r1" or not, because the test certificate also uses secp384r1 algorithm in the signature.

This is an exsiting test entry, and perahps filing a separate ticket if any change shall be made.

mpdonova · 2025-10-09T11:46:29Z

Can you add these new groups to test/jdk/javax/net/ssl/TLSCommon/NamedGroup.java?

haimaychao · 2025-10-13T17:07:13Z

New groups added to test/jdk/javax/net/ssl/TLSCommon/NamedGroup.java.

haimaychao · 2025-10-14T20:33:53Z

/csr

openjdk · 2025-10-14T20:34:57Z

@haimaychao an approved CSR request is already required for this pull request.

artur-oracle · 2025-10-14T22:16:59Z

test/jdk/javax/net/ssl/TLSCommon/NamedGroup.java

    FFDHE6144("ffdhe6144"),
    FFDHE8192("ffdhe8192");

+    X25519MLKEM768("X25519MLKEM768);


Invalid syntax: The matching quotation mark is missing,

artur-oracle · 2025-10-14T22:18:11Z

test/jdk/javax/net/ssl/TLSCommon/NamedGroup.java

    FFDHE8192("ffdhe8192");

+    X25519MLKEM768("X25519MLKEM768);
+    SecP256r1MLKEM768("SecP256r1MLKEM768");


Invalid syntax: enums should be comma-separated.

artur-oracle · 2025-10-14T23:02:01Z

src/java.base/share/classes/sun/security/ssl/NamedGroup.java

+            NamedParameterSpec.X25519MLKEM768,
+            "DH"),
+
+    SecP256r1MLKEM768(0x11eb, "SecP256r1MLKEM768",


Nit: the rest of named groups in this file are all upper-cased.

Changed hybrid groups to upper-case.

artur-oracle · 2025-10-14T23:06:19Z

src/java.base/share/classes/sun/security/ssl/ServerHello.java

+            // derives the secret after receiving the client's share (KA).
+            // However, this is changed for KEM: the server (as encapsulator)
+            // must perform both actions — derive the secret and generate the
+            // encapsulated message at the same time during SHKeyShareProducer.


Nit: during SHKeyShareProducer doesn't seem gramatically correct, did you mean during encapsulation in SHKeyShareProducer?

Yes, comment updated.

artur-oracle · 2025-10-14T23:11:28Z

src/java.base/share/classes/sun/security/ssl/ServerHello.java

+            // Traditional Key Agreement (KA):
+            //  Both peers perform similar operations: generate a public key,
+            //  send it, and compute a shared secret upon receiving the peer's
+            //  public key.


Nit: We use both public key and key share in this comment when describing Traditional Key Agreement. I think we should use only key share (as in RFC 8446) when describing a traditional key exchange to avoid any confusion with KEM's public key.

Fixed the comment.

artur-oracle · 2025-10-15T14:25:50Z

src/java.base/share/classes/java/security/spec/NamedParameterSpec.java

+     *
+     * @since 26
+     */
+    public static final NamedParameterSpec SecP384r1MLKEM1024


Nit: Let's make these all upper-case as well to keep things consistent across files.

artur-oracle · 2025-10-15T14:26:43Z

test/jdk/javax/net/ssl/TLSCommon/NamedGroup.java

+
+    X25519MLKEM768("X25519MLKEM768"),
+    SecP256r1MLKEM768("SecP256r1MLKEM768"),
+    SecP384r1MLKEM1024("SecP384r1MLKEM1024");


Nit: Make those uppercase.

We keep the actual name strings the same as in spec, the enum variables are to be uppercase for consistency.

XueleiFan · 2025-10-27T02:40:39Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+                }
+            }
+
+            throw new InvalidKeySpecException(keySpec.toString());


Please check null keySpec, and throw new InvalidKeySpecException("keySpec must not be null").

For non-null keySpec, I may use:
throw new InvalidKeySpecException(keySpec.getClass().getName() + " not supported.");

XueleiFan · 2025-10-27T02:49:39Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+    public record SecretKeyImpl(SecretKey k1, SecretKey k2) implements SecretKey {
+        @Override
+        public String getAlgorithm() {
+            return "Hybrid";


Will this name be added to CSR and Java Security Standard Algorithm Names doc?

XueleiFan · 2025-10-27T02:54:46Z

test/jdk/java/security/spec/TestNamedParameterSpec.java

        "ED25519", "ED448", "X25519", "X448",
        "ML_DSA_44", "ML_DSA_65", "ML_DSA_87",
        "ML_KEM_512", "ML_KEM_768", "ML_KEM_1024",
+        "X25519MLKEM768", "SecP256r1MLKEM768", "SecP384r1MLKEM1024",


Does it sound like an option to use the similar naming style like "ML_KEM_512" for the new names? for example, X25519MLKEM768 -> X25519_ML_KEM_768

XueleiFan · 2025-10-27T03:44:45Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+     */
+    private static KEM getKEM(String name) throws NoSuchAlgorithmException {
+        if (APS.isGenericEC(name) || APS.isXDH(name)) {
+            return KEM.getInstance("DH", DH.PROVIDER);


May I get more information about the hard-coded provider DH.PROVIDER? Could it be more general so that other KEM provider can also be used?

the DH provider is there give a KEM-like face on the NIST curves, x25519/448, etc., mainly for the purposes of assisting the work done within the hybrid KEM. The underlying work being done by the internal DH provider still ends up doing KeyAgreement, KeyPairGenerator and KeyFactory operations, and those should go through the usual provider selection process.

XueleiFan · 2025-10-27T03:59:03Z

src/java.base/share/classes/com/sun/crypto/provider/DH.java

+    // DH in its own private provider so we always getInstance from here.
+    public static final Provider PROVIDER = new ProviderImpl();
+
+    private static class ProviderImpl extends Provider {


Why not expose the Provider to public?

XueleiFan · 2025-10-27T04:02:51Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+        protected PublicKey engineGeneratePublic(KeySpec keySpec)
+                throws InvalidKeySpecException {
+            if (keySpec instanceof RawKeySpec rks) {
+                byte[] key = rks.getKeyArr();


null check on key may be missed. If the API expose to public in the future, the key length checking may be good before Arrays.copyOfRange.

XueleiFan · 2025-10-27T04:13:15Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+                        } catch (Exception e) {
+                            leftKey = left.generatePublic(new X509EncodedKeySpec(
+                                    leftKeyBytes));


Fall-back is not good for performance. Does it have to support two key spec?

XueleiFan · 2025-10-27T04:16:03Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+                left.initialize(leftSpec, random);
+                right.initialize(rightSpec, random);
+                initialized = true;
+            } catch (Exception e) {


It may be nice if not wrap InvalidParameterException twice.

XueleiFan · 2025-10-27T04:29:12Z

src/java.base/share/classes/sun/security/util/Hybrid.java

+            if (!initialized) {
+                try {
+                    left.initialize(leftSpec);
+                    right.initialize(rightSpec);
+                    initialized = true;
+                } catch (Exception e) {
+                    throw new ProviderException(e);
+                }
+            }


The initialization block could be saved if call initialize in the constructor. See sun/security/ec/ECKeyPairGenerator.java

haimaychao added 3 commits October 2, 2025 14:53

8314323: TLS 1.3 Hybrid Key Exchange

d5f88bb

Merge

ba5f6f3

8314323: TLS 1.3 Hybrid Key Exchange

c7ba12a

openjdk bot added the security [email protected] label Oct 2, 2025

haimaychao marked this pull request as ready for review October 2, 2025 23:59

openjdk bot added the rfr Pull request is ready for review label Oct 2, 2025

artur-oracle reviewed Oct 3, 2025

View reviewed changes

ecki reviewed Oct 3, 2025

View reviewed changes

Updates with review comments

3c0e91e

artur-oracle reviewed Oct 6, 2025

View reviewed changes

haimaychao added 2 commits October 13, 2025 08:58

Updates with review comments

47d804f

Rename NAMED_GROUP_KEM from PQC to KEM

5457a73

haimaychao changed the title ~~8314323: TLS 1.3 Hybrid Key Exchange~~ 8314323: Implement JEP 527: TLS 1.3 Hybrid Key Exchange Oct 13, 2025

openjdk bot added the csr Pull request needs approved CSR before integration label Oct 14, 2025

Call getProvider() in NamedGroup.java

39d2f1c

artur-oracle reviewed Oct 14, 2025

View reviewed changes

Fix typo in NamedGroup test

21c8dcc

artur-oracle reviewed Oct 14, 2025

View reviewed changes

haimaychao added 2 commits October 14, 2025 23:38

Updated comment in ServerHello and hybrid to upper-case in NamedGroup

c74cf9b

Revert changes to UseStrongDHSizes test as ffdhe6144/8192 added back

f98b538

artur-oracle reviewed Oct 15, 2025

View reviewed changes

XueleiFan reviewed Oct 27, 2025

View reviewed changes

Uh oh!

8314323: Implement JEP 527: TLS 1.3 Hybrid Key Exchange #27614

Are you sure you want to change the base?

8314323: Implement JEP 527: TLS 1.3 Hybrid Key Exchange #27614

Uh oh!

Conversation

haimaychao commented Oct 2, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Contributors

Reviewing

Uh oh!

bridgekeeper bot commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haimaychao commented Oct 2, 2025

Uh oh!

haimaychao commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025

Uh oh!

mlbridge bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haimaychao commented Oct 2, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 2, 2025 •

edited

Loading

mlbridge bot commented Oct 3, 2025 •

edited

Loading